Ensure ./x.py dist adheres to build.tools
#87282
Conversation
rust-highfive
added
the
S-waiting-on-review
|
Do you know if there was a regression? According to issue #86436 (Unable to build 1.53.0 from official tarball due to miri) this also does not work anymore with |
|
Hmm, this PR doesn't change |
|
Overall looks good -- I think the refactor I suggested could be nice, but is also fine to leave out from this PR. I think over the last few months I've started thinking about a new design for the default/should_run system anyway, which would likely address my suggestion in a slightly better way. So unless you're feeling particularly enthusiastic about the possible cleanup, let's leave it out for the time being. I would like the first comment about tools = [] addressed though, that should be easy. |
Mark-Simulacrum
added
S-waiting-on-author
|
Pushed the refactoring as well, so you can review it. I haven't tested it behaves correctly in all the cases, so let's wait until I do so before merging (reviewing is fine though). |
This comment has been minimized.
This comment has been minimized.
pietroalbini
force-pushed
the
refactor-extended
branch
from
f6202a5 to
c15e248
Compare
pietroalbini
added
S-waiting-on-review
The default is then overridden by `should_run`.
|
Ok fixed a bug I found while testing, the PR should now work as expected. r? @Mark-Simulacrum |
|
|
||
| // Avoid running steps contained in --exclude | ||
| for pathset in &should_run.paths { | ||
| if desc.is_excluded(self, pathset) { |
There was a problem hiding this comment.
Hm -- this is going to end up printing a bunch of times that things are / are not excluded, which seems a little annoying (i.e., when this is invoked multiple times). Maybe we should make it a ensure-style query that gets its result cached...
Seems OK for this PR though, that printing is noisy anyway.
|
@bors r+ |
|
📌 Commit 69f712c has been approved by |
bors
added
S-waiting-on-bors
Rollup of 13 pull requests Successful merges: - rust-lang#86183 (Change environment variable getters to error recoverably) - rust-lang#86439 (Remove `Ipv4Addr::is_ietf_protocol_assignment`) - rust-lang#86509 (Move `os_str_bytes` to `sys::unix`) - rust-lang#86593 (Partially stabilize `const_slice_first_last`) - rust-lang#86936 (Add documentation for `Ipv6MulticastScope`) - rust-lang#87282 (Ensure `./x.py dist` adheres to `build.tools`) - rust-lang#87468 (Update rustfmt) - rust-lang#87504 (Update mdbook.) - rust-lang#87608 (Remove unused field `Session.system_library_path`) - rust-lang#87629 (Consistent spelling of "adapter" in the standard library) - rust-lang#87633 (Update compiler_builtins to fix i128 shift/mul on thumbv6m) - rust-lang#87644 (Recommend `swap_remove` in `Vec::remove` docs) - rust-lang#87653 (mark a UB doctest as no_run) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
According to
config.toml.example, the way to produce dist artifacts for both the compiler and a subset of tools would be to enable the extended build and manually specify the list of tools to build:This works as expected for
./x.py buildand./x.py install, but not for./x.py dist. Before this PR./x.py distsimply ignored the contents ofbuild.tools, building just rustc/rustdoc ifbuild.extended = falseand all of the tools otherwise. This PR does two things:./x.py dist extendedto only build the tools defined inbuild.tools, ifbuild.toolsis not empty. The rest of the extended step was refactored to simplify the code.assert!(builder.config.extended)to prevent tools from being built withbuild.extended = false, tools are simply built by default depending onbuild.extendedandbuild.tools. This also enables to explicitly dist tools even withbuild.extended = false.This PR is best reviewed commit-by-commit.
Fixes #86436